Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverting toArray call, it is valid to pass in pre-alloc size #45

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

zsmoore
Copy link
Owner

@zsmoore zsmoore commented Jun 5, 2019

  • Please check if the PR fulfills these requirements
  • [N/A] Tests for the changes have been added (for bug fixes / features)
  • [ N/A] Docs have been added / updated (for bug fixes / features)

https://docs.oracle.com/javase/7/docs/api/java/util/Set.html#toArray(T[])

and If the set fits in the specified array, it is returned therein. Otherwise, a new array is allocated with the runtime type of the specified array and the size of this set.

Passing size is a valid thing to do.

  • What is the current behavior? (You can also link to an open issue here)
    Current behavior is the same as

(JsonFile[]) filesToLint.toArray()

  • What is the new behavior (if this is a feature change)?
    Array is optimized with same size as needed and no longer makes an unnecessary array of size 0 on each toArray call.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    None.

  • Other information:

@zsmoore zsmoore added the bug Something isn't working label Jun 5, 2019
@zsmoore zsmoore self-assigned this Jun 5, 2019
Copy link
Collaborator

@PawanHegde PawanHegde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is valid to pass in a fixed sized array, it is also just as valid (maybe more?) to pass in a zero-sized array as a type hint when you don't want a larger array. Our intention is to get an array of exactly the same size as the set. Passing 0 ensures this. There are no side-effects as far as it concerns our code. The link you cited also uses the example String[] y = x.toArray(new String[0]); instead of String[] y = x.toArray(new String[x.size()]);.

As far as performance is concerned, new String[0] is faster as I explained in PR #40 despite the JVM needing to create a throwaway empty array. So if you're concerned about performance, us fixing the size of the array will be slower than letting the JVM decide. Not to mention the extra function call to filesToLint.size() (which will also be really fast on an optimized JVM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants